Repository navigation
fix(traceAnalyzer): rename --popularityDacay to --popularityDecay - #381
Merged
Merged
Conversation
…as alias The popularity decay option was registered as --popularityDacay, so the correctly spelled --popularityDecay failed with "unrecognized option" (exit 64), and --help printed the misspelling. Register the option as --popularityDecay and keep the old spelling as a hidden argp alias (OPTION_ALIAS | OPTION_HIDDEN) so existing scripts keep working. Update doc/quickstart_traceAnalyzer.md to use --popularityDecay and drop the note that documented the typo. Add testCLI cases that run both spellings against cloudPhysicsIO.vscsi, check each exits 0 and writes cloudPhysicsIO.vscsi.popularityDecay_w300_obj, and check that --help lists --popularityDecay but not the hidden alias. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VD7LAJg16jcTU7MAhkm3G
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Only the correct spelling, --popularityDecay, is accepted now; the misspelled --popularityDacay is rejected as an unrecognized option. testCLI now covers just --popularityDecay and its --help entry. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015VD7LAJg16jcTU7MAhkm3G
1a1a11a
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b8431f9902
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
traceAnalyzerregistered the popularity decay option as--popularityDacay. Because of that typo, the correctly spelled option failed:--helpalso printed the misspelling, even though the task list in the help text already saidpopularityDecay.libCacheSim/bin/traceAnalyzer/cli_parser.cpp: the option is renamed to--popularityDecay. There is no alias, so--popularityDacayis now rejected as an unrecognized option (exit 64). Breaking for any script that used the misspelling.doc/quickstart_traceAnalyzer.md: the task table and the popularity decay example now use--popularityDecay, and the note explaining the typo is removed. No other file indoc/,README.mdorscripts/usedpopularityDacay.test/test_cli.sh(testCLI): new cases in the option parsing section:--popularityDecayagainstcloudPhysicsIO.vscsiand check that it exits 0 and writescloudPhysicsIO.vscsi.popularityDecay_w300_obj--helplists--popularityDecayThe run writes into its own directory, so the file check can't pass because of another run's output.
Type of change
How was it tested?
Checked by hand from
_build:--popularityDecayexits 0 and writescloudPhysicsIO.vscsi.popularityDecay_w300_obj.--popularityDacayfails withunrecognized option '--popularityDacay'(exit 64).--helpshows--popularityDecay.To confirm the new tests catch the bug, I rebuilt
traceAnalyzerfrom the originalcli_parser.cppand rantest_cli.sh: the new cases failed there and pass with this change.On this short sample trace, the
_objfile holds only its two header lines, because the trace ends before the default 1-day warmup. So the test checks that the option takes effect and the file is written, not what the analysis outputs.ctest --test-dir _build --output-on-failurepasses-Wall -Wextra -Werror)Checklist
clang-format(or the pre-commit hook fromscripts/setup_hooks.sh)🤖 Generated with Claude Code
https://claude.ai/code/session_015VD7LAJg16jcTU7MAhkm3G